Enable Block-Max WAND dynamic pruning for FunctionScoreQuery via DocValuesSkipper (+422% QPS) - #16431
Enable Block-Max WAND dynamic pruning for FunctionScoreQuery via DocValuesSkipper (+422% QPS)#16431rajat315315 wants to merge 15 commits into
Conversation
|
Hi, there is one big problem, otherwise we would have enabled this for function queries already!: It won't work correctly if the function modifies the score in a way that it is not an increasing function, e.g, a function like |
|
Thanks for raising this crucial point regarding monotonicity! I have updated the PR to handle both monotonically increasing and monotonically decreasing functions while maintaining strict safety for arbitrary expressions: 1. 100% Sound & Safe Fallback (
|
|
I don't see how you can " guarantee[s] zero correctness bugs " - sounds like a statement from an overambitious AI! In particular, if someone implements a scoring function like |
|
@msokolov |
I understood it like this: The default is to use INFINITY when returning if it is maxScore ready. If you implement a sinus function and implement maxScore for it, it would break. But then it's your own problem, because you implemented maxScore support for it. What I did not yet undertsood: Where are the defaults (increasing/decreasing) as in the statement above for What happens if you use a ValueSource with an increasing function and you wrap it with another decreasing function (nesting)? There should be clear paths so it is also safe for the user. Otherwise I would be against such an optimization. |
I have attached a test specifying
The caller constructing a // Clean, transparent caller API (no boolean flags needed):
Query query = new FunctionScoreQuery(baseQuery, doubleValuesSource);
When nesting functions
If any layer in the nesting chain is non-monotonic or lacks skip index information, it returns |
|
I think there's one problem, in your example with a "simple function" inline (lambda): DoubleValuesSource valSource = DoubleValuesSource.fromField("val", v -> 10000.0 / v, false);The And on top: the version without the boolean by default assumes no direction and we undeprecate it. |
|
I'd suggest to replace the boolean with an enum: |
|
I guess you are correct, when monotonicity is unknown or un-supplied, we need to assume score to be Float.POSITIVE_INFINITY and go over each Doc in the block. EDIT: On the other hand, I'm running luceneutil benchmark on top my PR just to know performance improvement on a real world dataset. I'm still using 4096 doc blocks and will create a separate issue to bring it down to 128 wide blocks. |
| /** Monotonicity direction of a decoder function */ | ||
| public enum Monotonicity { | ||
| /** Increasing function */ | ||
| INCREASING, |
There was a problem hiding this comment.
I think we do not need strictly increasing, rather NONDECREASING and NONINCREASING, right? what if the function is constant?
There was a problem hiding this comment.
Also, if the function is a composite function, like sum, it is non-increasing when both of its inputs are, but non-decreasing when both of its inputs are, but when one is decreasing and the other one increasing, it is undefined. I don't know how we can really expect this to work with general functions
There was a problem hiding this comment.
I don't like to have "NON" in the name, this makes it hard to use. INCREASING should be fine, because unless it is strictly increasing it means that the values go up. A constant function is fine, you should be able to chooseany of INCREASING or DECREASING, correct?
There was a problem hiding this comment.
I think in those cases where we are uncertain about the monotonicity of a function, we can use Monotonicity.NONE.
There was a problem hiding this comment.
So I think terms are correct. Constant function is allowed and can be classified as both increasing or decreasing. It would also work fine in our code - you can pass both enum constants, because in case of a constant function the getMinScore() and getMaxScore() would both return the same value (the constant), so the result for a block with constant function is same for both increasing or decreasing.
There was a problem hiding this comment.
In case of constant scoring, my view-point is a little different.
Since all docs have the same score, we need to evaluate each and every doc. We can't prune any block.
In that case, we might need to use, Monotonicity.NONE
There was a problem hiding this comment.
I think it would still work if it is constant. In that case it would see the first doc with the constant value. After that it asks to skip, but provides the constant value. The code then needs to revisit all docs, because the comparison to min and max score needs to revisit all docs with exact same score. It can only exclude docs with larger or less value (that don't exist).
Maybe add a test. 😜
There was a problem hiding this comment.
thanks for sharing the definitions, OK by me to drop the NON
|
I have performed benchmarking on wikimedia dataset using luceneutil and below are the findings. The benchmark evaluates 20 distinct query permutations formed by combining 4 scoring function categories with 5 query template categories across 100 query instances per category (total: 1,936 task iterations across 5 JVM processes). A. Scoring Function Categories
B. Query Template Categories
3. Detailed Benchmark Performance Results
4. Key Performance Takeaways
|
uschindler
left a comment
There was a problem hiding this comment.
Looks good to me with the new API shapes.
|
Do you think this should also be backported or let us get it into Lucene 11 only. Please add a changes entry. I will wait for more time and think about the monotonicy again and try to figure out if everything works. In addition it would be good to add a test that checks witha constent function ( |


Resolves: #16429
Description
Currently,
FunctionScoreQueryhardcodesgetMaxScore(int upTo)toFloat.POSITIVE_INFINITYinside itsFilterScorerimplementation:Because
getMaxScore()returnsFloat.POSITIVE_INFINITY, Block-Max WAND (BMW) dynamic block pruning is disabled for function and script queries. Lucene is forced to evaluateDoubleValuesSourceand script bytecode on every single matching document doc-by-doc.Additionally, Lucene's default
DocValuesSkipperblock interval is 4,096 documents (DEFAULT_SKIP_INDEX_INTERVAL_SIZE = 4096), which is too coarse for fine-grained WAND score skipping.This PR:
FunctionScoreQueryandDoubleValuesSourceby querying the segment'sDocValuesSkipper.DEFAULT_SKIP_INDEX_INTERVAL_SIZEinLucene90DocValuesFormat.javafrom 4096 to 128 documents to unlock fine-grained WAND block skipping.Proposed Changes
1.
Lucene90DocValuesFormat.java(lucene/core)DEFAULT_SKIP_INDEX_INTERVAL_SIZE = 128(down from 4096) to write fine-grained 128-doc skip index blocks by default for numeric DocValues.2.
DoubleValues.java&DoubleValuesSource.java(lucene/core)public int advanceShallow(int target)andpublic float getMaxScore(int upTo)toDoubleValuesandDoubleValuesSource.FieldValuesSource(fromField,fromLongField,fromDoubleField, etc.) to queryLeafReaderContext.reader().getDocValuesSkipper(field):advanceShallow(target)delegates toskipper.advance(target)to advance shallow block boundaries.getMaxScore(upTo)returns(float) decoder.applyAsDouble(skipper.maxValue(0))when block bounds are available.3.
FunctionScoreQuery.java(lucene/queries)FunctionScoreWeight.scorerSupplier()so thatFilterScorer.getMaxScore(upTo)andadvanceShallow(target)delegate directly toscores.getMaxScore(upTo)andscores.advanceShallow(target)instead of returningFloat.POSITIVE_INFINITY.4. Unit Tests (
TestFunctionScoreQuery.java)Added unit test coverage in
TestFunctionScoreQuery.java:testMaxScoreDelegationWithDocValuesSkipper(): Verifies thatadvanceShallowandgetMaxScorequeryDocValuesSkipperand return finite score upper bounds.testMaxScorePruningTopDocs(): Verifies top-Benchmark Results (10M Docs, Top 100 Hits)
We benchmarked this optimization on a 10 Million document index ($N = 10,000,000$ , $1,000,000$ matching documents at $10%$ match density, top-$100$ target hits) on OpenJDK 25:
Test Suite Verification
All 507 unit tests in
lucene-queriespass cleanly:./gradlew :lucene:queries:test # BUILD SUCCESSFUL: 507 tests, 5 skipped, 0 failurescc @jpountz @romseygeek @mikemccand @uschindler